Skip to content

fix: file extension is not added to imported files when using NodeNext moduleResolution - #3361

Merged
melloware merged 3 commits into
orval-labs:masterfrom
rkday-pro:iss2284
May 15, 2026
Merged

fix: file extension is not added to imported files when using NodeNext moduleResolution#3361
melloware merged 3 commits into
orval-labs:masterfrom
rkday-pro:iss2284

Conversation

@rkday-pro

@rkday-pro rkday-pro commented May 15, 2026

Copy link
Copy Markdown
Contributor

Fixes #2284.

This basically takes the suggestion from that issue and runs with it:

Is it possible for orval to respect more options from tsconfig.json, and use .js in case just moduleResolution (module) is set to ESNext, and .ts in case both moduleResolution set to ESNext and allowImportingTsExtensions set to true?

  • interface Tsconfig now includes the relevant options
  • getImportExtension no longer just strips .ts - it takes the tsconfig as an argument and adds .js when those options are set
  • the tsconfig has to be plumbed through to more places, but I think that's unavoidable

I've run this in my project, where the orval-generated files previously failed typechecking with error TS2835: Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean './datapoint.js'?, and they now pass.

Summary by CodeRabbit

  • New Features

    • Generated imports now respect additional TypeScript compiler options (module, moduleResolution, allowImportingTsExtensions) when resolving emitted extensions.
    • Import path extensions are computed consistently and can preserve .ts-style extensions when allowed.
    • Schema generation and index exports now emit imports using the configured extension logic.
  • Tests

    • Added tests covering import/export suffix behavior across TypeScript configuration scenarios.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2e1d96d1-b574-432d-b08f-f2634e1aa33a

📥 Commits

Reviewing files that changed from the base of the PR and between c250bf6 and 7b5548c.

📒 Files selected for processing (2)
  • packages/core/src/utils/tsconfig.test.ts
  • packages/core/src/utils/tsconfig.ts
✅ Files skipped from review due to trivial changes (1)
  • packages/core/src/utils/tsconfig.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/core/src/utils/tsconfig.ts

📝 Walkthrough

Walkthrough

Threads tsconfig-driven import-extension logic through codegen: adds getImportExtension, extends Tsconfig types, passes computed importExtension into generateImports and schema writers, updates orchestration, and adds tests covering NodeNext and allowImportingTsExtensions behaviors.

Changes

Import extension computation and threading

Layer / File(s) Summary
TypeScript compiler options type definitions
packages/core/src/types.ts
Tsconfig.compilerOptions adds optional module, moduleResolution, and allowImportingTsExtensions fields.
Import extension computation utility
packages/core/src/utils/tsconfig.ts, packages/core/src/utils/tsconfig.test.ts
New getImportExtension(fileExtension, tsconfig?) detects NodeNext/Node16 and maps source extensions to emitted extensions or preserves the configured extension when allowed; tests cover behavior.
Import generator option
packages/core/src/generators/imports.ts
GenerateImportsOptions gains optional importExtension and generated relative import specifiers append this extension.
Schema writer integration and canonical path rewriting
packages/core/src/writers/schemas.ts
Replaces local extension helper with getImportExtension, threads tsconfig through fixSchemaImports, fixCrossDirectoryImports, fixRegularSchemaImports, normalizeCanonicalImportPaths, and passes computed importExtension into generateImports; index export suffix generation now uses the shared utility.
Tests for tsconfig-driven extension behavior
packages/core/src/writers/schemas.test.ts
Adds tests asserting .js suffixed imports when module: 'NodeNext' and .ts preservation when allowImportingTsExtensions: true with module: 'NodeNext'.
Builder and orchestration updates
packages/core/src/writers/generate-imports-for-builder.ts, packages/orval/src/write-specs.ts
generateImportsForBuilder uses getImportExtension; writeSpecs propagates output.tsconfig into schema import-fixing and writeSchemas invocations across branches.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Possibly related PRs

  • orval-labs/orval#3214: Similar edits in canonical import path normalization and extension handling in packages/core/src/writers/schemas.ts.
  • orval-labs/orval#3253: Related changes affecting generateImportsForBuilder and import path normalization.

Suggested labels

bug

Suggested reviewers

  • melloware

Poem

🐇 I stitched tsconfig through each generated line,
mapping .ts to .js where NodeNext asks it be so.
When allowImportingTsExtensions whispers "keep mine",
I tuck the .ts back in, neat as fresh snow.
Hooray — imports aligned, the schema garden grows!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main bug fix: that file extensions are now properly added to imported files when using NodeNext moduleResolution, which is the core objective of this PR.
Linked Issues check ✅ Passed The PR fully addresses issue #2284 by adding tsconfig compilerOptions support, implementing getImportExtension to respect module/moduleResolution and allowImportingTsExtensions, and threading tsconfig through import resolution for both mutator and model imports.
Out of Scope Changes check ✅ Passed All changes directly support the core objective of fixing file extension handling based on tsconfig compilerOptions; no unrelated refactoring or feature additions detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
packages/core/src/writers/schemas.ts (1)

225-227: 💤 Low value

Consider adding a clarifying comment for the fallback branch.

The fallback .replace(/\.ts$/, '') handles edge cases where relative doesn't end with fileExtension. Since canonical.importPath is constructed via getPath with fileExtension, the primary branch should handle most cases. A brief comment explaining when the fallback is needed would improve code clarity.

Example:

const withoutFileExtension = relative.endsWith(fileExtension)
  ? relative.slice(0, -fileExtension.length)
  : relative.replace(/\.ts$/, ''); // Fallback for legacy paths with hardcoded .ts extension
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/writers/schemas.ts` around lines 225 - 227, Add a short
clarifying comment on the fallback branch for the withoutFileExtension
computation: explain that relative.endsWith(fileExtension) handles normal paths
produced by getPath (used to build canonical.importPath) and the else branch
(relative.replace(/\.ts$/, '')) is a defensive fallback for legacy or hardcoded
".ts" paths that don't match the current fileExtension; place the comment next
to the else branch referencing withoutFileExtension, relative, fileExtension,
and canonical.importPath for context.
packages/core/src/utils/tsconfig.ts (1)

32-34: ⚡ Quick win

Incomplete extension mapping for NodeNext/Node16 module resolution.

The current implementation only converts .ts.js for NodeNext/Node16 module resolution. According to TypeScript's module resolution rules, additional extensions should also be mapped:

  • .tsx.jsx
  • .mts.mjs
  • .cts.cjs

While Orval likely generates only .ts files currently, adding complete support makes this utility more robust and prevents edge-case bugs if other extensions are introduced later.

♻️ Proposed fix to handle all TypeScript extensions
  if (
    (module && NODE_NEXT_MODULES.has(module)) ||
    (moduleResolution && NODE_NEXT_MODULES.has(moduleResolution))
  ) {
-    return fileExtension.endsWith('.ts')
-      ? `${fileExtension.slice(0, -3)}.js`
-      : fileExtension;
+    if (fileExtension.endsWith('.ts')) {
+      return `${fileExtension.slice(0, -3)}.js`;
+    }
+    if (fileExtension.endsWith('.tsx')) {
+      return `${fileExtension.slice(0, -4)}.jsx`;
+    }
+    if (fileExtension.endsWith('.mts')) {
+      return `${fileExtension.slice(0, -4)}.mjs`;
+    }
+    if (fileExtension.endsWith('.cts')) {
+      return `${fileExtension.slice(0, -4)}.cjs`;
+    }
+    return fileExtension;
  }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/utils/tsconfig.ts` around lines 32 - 34, The current return
expression only maps `.ts`→`.js`; update the extension mapping logic (where
`fileExtension` is used) to handle the NodeNext/Node16 mappings as well:
`.ts`→`.js`, `.tsx`→`.jsx`, `.mts`→`.mjs`, and `.cts`→`.cjs` (e.g. replace the
simple slice with a small mapping or switch that checks `fileExtension` and
returns the corresponding mapped extension, leaving other extensions unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/core/src/utils/tsconfig.ts`:
- Around line 32-34: The current return expression only maps `.ts`→`.js`; update
the extension mapping logic (where `fileExtension` is used) to handle the
NodeNext/Node16 mappings as well: `.ts`→`.js`, `.tsx`→`.jsx`, `.mts`→`.mjs`, and
`.cts`→`.cjs` (e.g. replace the simple slice with a small mapping or switch that
checks `fileExtension` and returns the corresponding mapped extension, leaving
other extensions unchanged).

In `@packages/core/src/writers/schemas.ts`:
- Around line 225-227: Add a short clarifying comment on the fallback branch for
the withoutFileExtension computation: explain that
relative.endsWith(fileExtension) handles normal paths produced by getPath (used
to build canonical.importPath) and the else branch (relative.replace(/\.ts$/,
'')) is a defensive fallback for legacy or hardcoded ".ts" paths that don't
match the current fileExtension; place the comment next to the else branch
referencing withoutFileExtension, relative, fileExtension, and
canonical.importPath for context.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3be8895c-0871-430d-9f61-0bf05c8e012c

📥 Commits

Reviewing files that changed from the base of the PR and between 06c44c1 and 2f1d068.

📒 Files selected for processing (7)
  • packages/core/src/generators/imports.ts
  • packages/core/src/types.ts
  • packages/core/src/utils/tsconfig.ts
  • packages/core/src/writers/generate-imports-for-builder.ts
  • packages/core/src/writers/schemas.test.ts
  • packages/core/src/writers/schemas.ts
  • packages/orval/src/write-specs.ts

@melloware melloware left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build is failing

@rkday-pro
rkday-pro requested a review from melloware May 15, 2026 20:22
@melloware melloware added the bug Something isn't working label May 15, 2026
@melloware
melloware merged commit e3b888e into orval-labs:master May 15, 2026
5 checks passed
@mauriceackel

Copy link
Copy Markdown
Contributor

@rkday-pro, thank you so much for this PR. I just realized that there is one place missing for change to take action and this is when writing zod specs. In particular, this line needs the changes to be applied too.

Any chance this can be added? I'd assume this could be labeled as a bug.

CC: @melloware

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

File extension is not added to imported files when using NodeNext moduleResolution

3 participants